controlplane/src/core/repositories/FeatureFlagRepository.ts (1)

495-512: Avoid 2N+1 lookups after paginating.

This path paginates in SQL, but each returned row still does byTargetId() and byId() serially. With the current 50-item cap, one page can still trigger 100+ extra DB queries. Folding the missing fields into the main query, or batch-loading them by ID, would keep the new endpoint’s latency predictable.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@controlplane/src/core/repositories/FeatureFlagRepository.ts` around lines 495
- 512, In FeatureFlagRepository where featureSubgraphTargets is iterated, avoid
N+1 by collecting all needed IDs (all targetId values and all baseSubgraphId
values from featureSubgraphTargets) and batch-loading them once (e.g., using
subgraphRepo.byIds or a new subgraphRepo.byTargetIds/byIds method) before the
loop; then map the returned subgraph rows by id/targetId and populate
featureSubgraphs from those maps (or alternatively fold the missing fields into
the primary SQL with joins) instead of calling subgraphRepo.byTargetId and
subgraphRepo.byId for each item.

controlplane/test/feature-flag/get-feature-flags-in-latest-composition-by-federated-graph.test.ts (1)

65-66: Assert the exact result set here.

This setup creates one eligible flag, but toBeGreaterThanOrEqual(1) plus some(...) still allows duplicates or unrelated extras. Tightening this to the exact returned IDs/names—or adding a second ineligible flag to the fixture—would actually lock down the “only flags in the latest composition” contract.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@controlplane/test/feature-flag/get-feature-flags-in-latest-composition-by-federated-graph.test.ts`
around lines 65 - 66, The test currently uses a loose assertion
(expect(resp.featureFlags.length).toBeGreaterThanOrEqual(1) plus some(...)), so
tighten it to assert the exact returned set: either change the length assertion
to expect(resp.featureFlags.length).toBe(1) and assert the single entry matches
flagName (e.g., resp.featureFlags[0].name === flagName) or replace both
assertions with a deterministic comparison of names/ids (e.g., compare
resp.featureFlags.map(f=>f.name) to [flagName]); alternatively, add a second
ineligible flag to the fixture and assert the response contains only the
eligible flag(s) to lock down the “only flags in the latest composition”
contract.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

studio/src/pages/[organizationSlug]/[namespace]/graph/[slug]/playground.tsx (1)

745-763: ⚠️ Potential issue | 🟠 Major

Prevent feature-flag mode from loading SDL before flag resolution.

Line 756 still executes the SDL query before the selected flag name exists (Line 761), so feature-flag mode can fetch base schema first and re-fetch after flags load. Also, the flag lookup is duplicated in three places (Line 761, Line 1012, Line 1067), which risks drift.

💡 Proposed fix

   const { data: compositionFlagsData } = useQuery(
     getFeatureFlagsInLatestCompositionByFederatedGraph,
@@
     },
   );
+  const selectedFeatureFlagName =
+    type === 'featureFlag'
+      ? (compositionFlagsData?.featureFlags ?? []).find((f) => f.id === loadSchemaGraphId)?.name
+      : undefined;
 
-  const { data, isLoading: isLoadingGraphSchema } = useQuery(getFederatedGraphSDLByName, {
-    name: graphContext?.graph?.name,
-    namespace: graphContext?.graph?.namespace,
-    featureFlagName:
-      type === 'featureFlag'
-        ? (compositionFlagsData?.featureFlags ?? []).find((f) => f.id === loadSchemaGraphId)?.name
-        : undefined,
-  });
+  const { data, isLoading: isLoadingGraphSchema } = useQuery(
+    getFederatedGraphSDLByName,
+    {
+      name: graphContext?.graph?.name,
+      namespace: graphContext?.graph?.namespace,
+      featureFlagName: selectedFeatureFlagName,
+    },
+    {
+      enabled: !!graphContext?.graph?.name && (type !== 'featureFlag' || !!selectedFeatureFlagName),
+    },
+  );
@@
           graphContext?.graph?.id || '',
-          type === 'featureFlag'
-            ? (compositionFlagsData?.featureFlags ?? []).find((f) => f.id === loadSchemaGraphId)?.name
-            : undefined,
+          selectedFeatureFlagName,
         ),
@@
-        if (type === 'featureFlag') {
-          const featureFlag = (compositionFlagsData?.featureFlags ?? []).find((f) => f.id === loadSchemaGraphId);
-          if (featureFlag) {
-            requestHeaders['X-Feature-Flag'] = featureFlag.name;
-          }
+        if (selectedFeatureFlagName) {
+          requestHeaders['X-Feature-Flag'] = selectedFeatureFlagName;
         }
@@
-    compositionFlagsData?.featureFlags,
+    selectedFeatureFlagName,
@@
-    compositionFlagsData?.featureFlags,
+    selectedFeatureFlagName,

Also applies to: 1011-1013, 1066-1070

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@studio/src/pages/`[organizationSlug]/[namespace]/graph/[slug]/playground.tsx
around lines 745 - 763, Prevent the SDL query from running in feature-flag mode
until the flags are loaded and the selected flag is resolved: compute a single
selectedFlagName variable by deriving it once from compositionFlagsData,
loadSchemaGraphId and use that name wherever needed (replace the repeated .find
usages at the three spots), and pass featureFlagName: selectedFlagName to the
getFederatedGraphSDLByName useQuery; also set the useQuery enabled flag to only
run when either type !== 'featureFlag' or selectedFlagName is truthy so the base
SDL is not fetched prematurely.

controlplane/src/core/repositories/FeatureFlagRepository.ts (1)

437-439: ⚠️ Potential issue | 🟡 Minor

Keep feature-subgraph ID search working.

This new path only filters on targets.name, so UUID searches that already work in getFeatureSubgraphs() stop returning results here.

💡 Proposed fix

     if (query) {
-      conditions.push(like(targets.name, `%${query}%`));
+      conditions.push(isValidUuid(query) ? eq(subgraphs.id, query) : like(targets.name, `%${query}%`));
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@controlplane/src/core/repositories/FeatureFlagRepository.ts` around lines 437
- 439, The current query filter only matches targets.name so UUID searches stop
returning results; in the method that builds conditions (where you have if
(query) { conditions.push(like(targets.name, `%${query}%`)); }) update it to
also check the feature-subgraph id—e.g., add a second condition on targets.id
(exact match) or OR a like on targets.id when query looks like a UUID—so
getFeatureSubgraphs()/the same query path will return results for ID searches as
well; modify the conditions.push call(s) to include targets.id alongside
targets.name (and optionally detect UUID format to prefer exact match).

controlplane/src/core/bufservices/feature-flag/getFeatureFlagsInLatestCompositionByFederatedGraph.ts (1)

69-83: ⚠️ Potential issue | 🟠 Major

This returns current flag state, not the latest composed snapshot.

getFeatureFlagSchemaVersionsByBaseSchemaVersion() gives you the persisted composition membership, but this loop throws that record away and reloads the mutable live flag by featureFlagId. If a flag is disabled, relabeled, or deleted after that composition, this RPC will drift from what the latest composition actually contained.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@controlplane/src/core/bufservices/feature-flag/getFeatureFlagsInLatestCompositionByFederatedGraph.ts`
around lines 69 - 83, The code currently discards the composition membership
returned by getFeatureFlagSchemaVersionsByBaseSchemaVersion
(ffsInLatestValidComposition) and reloads live flags via
featureFlagRepo.getFeatureFlagById, which returns the current mutable state
instead of the snapshot that was part of the composition; change the lookup to
fetch the versioned/snapshot data tied to the composition record (e.g., use the
schema-version-scoped accessor instead of getFeatureFlagById) — for example call
a repository method that returns the feature-flag snapshot by featureFlagId +
base/schema version (or use the data already present on the ff record) so you
populate featureFlags from the composed snapshot (reference symbols:
getFeatureFlagSchemaVersionsByBaseSchemaVersion, ffsInLatestValidComposition,
ff.featureFlagId, featureFlagRepo.getFeatureFlagById, featureFlags).

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

controlplane/src/core/repositories/FeatureFlagRepository.ts (1)

467-468: 🛠️ Refactor suggestion | 🟠 Major

Add a stable tiebreaker to the paginated ordering.

ORDER BY targets.createdAt alone is not deterministic when multiple rows share the same timestamp, so offset-based paging can duplicate or skip feature subgraphs between pages.

💡 Proposed fix

       .innerJoin(namespaces, eq(namespaces.id, targets.namespaceId))
       .where(and(...conditions, eq(subgraphsToFederatedGraph.federatedGraphId, federatedGraphId)))
-      .orderBy(asc(targets.createdAt));
+      .orderBy(asc(targets.createdAt), asc(subgraphs.id));

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@controlplane/src/core/repositories/FeatureFlagRepository.ts` around lines 467
- 468, The ORDER BY in FeatureFlagRepository using
.orderBy(asc(targets.createdAt)) is not deterministic and can cause
duplicates/skips during offset paging; update the query that builds the ordering
(the call chaining ending with .orderBy(...)) to add a stable tiebreaker such as
the primary key (e.g., targets.id or targets.pk) and include it with the same
sort direction (e.g., .orderBy(asc(targets.createdAt), asc(targets.id))) so rows
with identical createdAt values are consistently ordered.

controlplane/src/core/repositories/FeatureFlagRepository.ts (2)

251-253: Redundant condition can be simplified.

Since isOrganizationViewer is defined as isOrganizationAdminOrDeveloper || this.roles.includes('organization-viewer') (per RBACEvaluator.ts:80), checking isOrganizationAdminOrDeveloper separately is redundant.

💡 Proposed simplification

   private applyRbacConditionsToQuery(rbac: RBACEvaluator | undefined, conditions: (SQL<unknown> | undefined)[]) {
-    if (!rbac || rbac.isOrganizationAdminOrDeveloper || rbac.isOrganizationViewer) {
+    if (!rbac || rbac.isOrganizationViewer) {
       return true;
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@controlplane/src/core/repositories/FeatureFlagRepository.ts` around lines 251
- 253, The condition in FeatureFlagRepository (variable rbac) redundantly checks
rbac.isOrganizationAdminOrDeveloper and rbac.isOrganizationViewer; since
RBACEvaluator's isOrganizationViewer already includes
isOrganizationAdminOrDeveloper, simplify the guard to only check for absence of
rbac or rbac.isOrganizationViewer (i.e., replace the current if (!rbac ||
rbac.isOrganizationAdminOrDeveloper || rbac.isOrganizationViewer) return true;
with if (!rbac || rbac.isOrganizationViewer) return true;), referencing the
RBACEvaluator properties to justify the change.

---

489-505: N+1 query pattern could be optimized with batch fetching.

Each iteration executes two separate DB queries (byTargetId and byId). With a page of N results, this generates 2N additional queries. Consider collecting all targetIds and baseSubgraphIds upfront and fetching them in bulk.

💡 Proposed batch approach

     const featureSubgraphTargets = await baseQuery.execute();
 
+    // Batch fetch all required data
+    const targetIds = featureSubgraphTargets.map((f) => f.targetId);
+    const baseSubgraphIds = [...new Set(featureSubgraphTargets.map((f) => f.baseSubgraphId))];
+    
+    const [subgraphsByTarget, baseSubgraphsById] = await Promise.all([
+      subgraphRepo.byTargetIds(targetIds),  // Would need to add this batch method
+      subgraphRepo.byIds(baseSubgraphIds),  // Would need to add this batch method
+    ]);
+    
+    const targetMap = new Map(subgraphsByTarget.map((s) => [s.targetId, s]));
+    const baseMap = new Map(baseSubgraphsById.map((s) => [s.id, s]));
+
     const featureSubgraphs: FeatureSubgraphDTO[] = [];
     for (const f of featureSubgraphTargets) {
-      const fs = await subgraphRepo.byTargetId(f.targetId);
+      const fs = targetMap.get(f.targetId);
       if (!fs) {
         continue;
       }
 
-      const baseSubgraph = await subgraphRepo.byId(f.baseSubgraphId);
+      const baseSubgraph = baseMap.get(f.baseSubgraphId);
       if (!baseSubgraph) {
         continue;
       }

Note: This would require adding batch methods to SubgraphRepository if they don't exist.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@controlplane/src/core/repositories/FeatureFlagRepository.ts` around lines 489
- 505, The loop over featureSubgraphTargets causes an N+1 query pattern by
calling subgraphRepo.byTargetId and subgraphRepo.byId per item; collect all
targetIds and baseSubgraphIds from featureSubgraphTargets first, add or use
batch methods on SubgraphRepository (e.g., byTargetIds(targetIds[]) and
byIds(baseIds[])) to fetch maps of Subgraph objects in two bulk queries, then
iterate featureSubgraphTargets and look up fs and baseSubgraph from those maps
when building featureSubgraphs (replace the per-item calls to byTargetId and
byId).

controlplane/src/core/repositories/SubgraphRepository.ts (1)

685-685: Add an explicit return type to the new static helper.

Callers branch on this value as a boolean, so : boolean should be part of the signature instead of being inferred.

Suggested change

-  static applyRbacConditionsToQuery(rbac: RBACEvaluator | undefined, conditions: (SQL<unknown> | undefined)[]) {
+  static applyRbacConditionsToQuery(
+    rbac: RBACEvaluator | undefined,
+    conditions: (SQL<unknown> | undefined)[],
+  ): boolean {

As per coding guidelines, **/*.{ts,tsx}: Use explicit type annotations for function parameters and return types in TypeScript.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@controlplane/src/core/repositories/SubgraphRepository.ts` at line 685, The
static helper applyRbacConditionsToQuery currently relies on an inferred return
type but callers branch on it as a boolean—update its signature to explicitly
declare a boolean return type by adding ": boolean" to the method declaration
(static applyRbacConditionsToQuery(rbac: RBACEvaluator | undefined, conditions:
(SQL<unknown> | undefined)[]): boolean) and ensure the implementation returns a
boolean value in all code paths to satisfy the explicit annotation.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes